-
Notifications
You must be signed in to change notification settings - Fork 13.5k
bootstrap.py: add lockfile #143854
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
bootstrap.py: add lockfile #143854
Conversation
rustbot has assigned @Mark-Simulacrum. Use |
This comment has been minimized.
This comment has been minimized.
f033e32
to
9d46820
Compare
This comment has been minimized.
This comment has been minimized.
9d46820
to
4ac0afb
Compare
the rust part of bootstrap already has a lockfile, but that's not good enough, the python side also shouldn't race with itself. fixes RUST-143756
4ac0afb
to
1c6986f
Compare
run(args, env=env, verbose=build.verbose, is_bootstrap=True) | ||
finally: | ||
# always remove the lockfile | ||
os.remove(lock_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we remove the lockfile earlier? It feels a bit odd to remove it only after (IIUC) we run the Rust part -- then the Rust lockfile is never considered, right? And the Rust part has far more extensive code (e.g., we print the PID and such, IIRC)...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't feel like a good idea.
The files modified by the python side are often read by the rust side. If we remove the lockfile early, the second python command could start modifying files that are read by the rust command.
Sure, we could write bootstrap very particularly to work around this... but that seems like a waste of effort (that would need to be maintained continuously to prevent bugs) for a very niche upside.
More reasonable I think would be just porting these features to the python side.
the rust part of bootstrap already has a lockfile, but that's not good enough,
the python side also shouldn't race with itself.
fixes #143756